Add OnPacketReceived to pseudocode #3653
Conversation
Pseudocode for re-arming the PTO when "If no additional data can be sent, the server's PTO timer MUST NOT be armed until datagrams have been received from the client, because packets sent on PTO count against the anti-amplification limit. "
ianswett
added
editorial
There was a problem hiding this comment.
Thanks for doing this. I have one tweak that I think you need and maybe a tweak that you want.
The first is about the argument and what information is passed. I think that this is just a notification from the main packet processing code, so it doesn't need to include packet processing.
The second is more involved and would involve adding more state variables.
draft-ietf-quic-recovery.md
Outdated
| Psuedocode for OnPacketReceived follows: | ||
|
|
||
| ~~~ | ||
| OnPacketReceived(packet): |
There was a problem hiding this comment.
I would drop the argument to this. Because this needs to be a datagram not a packet.
That means the function at the end should be called ProcessPackets(datagram) if you keep it. But I would suggest that you don't need to do packet processing in this logic: this is just a function where the loss recovery code is told about what is happening in the main code.
There was a problem hiding this comment.
+1. You don't need to process packets in here, just say up above that this function is called even if the received packet is undecryptable.
There was a problem hiding this comment.
Method renamed and comment updated.
draft-ietf-quic-recovery.md
Outdated
| Psuedocode for OnPacketReceived follows: | ||
|
|
||
| ~~~ | ||
| OnPacketReceived(packet): |
There was a problem hiding this comment.
+1. You don't need to process packets in here, just say up above that this function is called even if the received packet is undecryptable.
draft-ietf-quic-recovery.md
Outdated
| Psuedocode for OnPacketReceived follows: | ||
|
|
||
| ~~~ | ||
| OnPacketReceived(packet): |
There was a problem hiding this comment.
Method renamed and comment updated.
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Co-authored-by: Jana Iyengar <jri.ietf@gmail.com>
draft-ietf-quic-recovery.md
Outdated
| When a server is blocked by anti-amplification limits, receiving | ||
| a datagram unblocks it, even if none of the packets in the | ||
| datagram are successfully processed. In such a case, the PTO | ||
| timer may need to be armed. |
There was a problem hiding this comment.
When would it not be armed?
There was a problem hiding this comment.
I thought a padded ACK-only packet could hit the limit without bytes in flight, but I realize that each full-sized ACK allows 3 packets to be sent. Updating text.
Pseudocode for re-arming the PTO when "If no additional data can be sent, the server's PTO timer MUST NOT be armed until datagrams have been received from the client, because packets sent on PTO count against the anti-amplification limit. "
I'll note that OnPacketReceived actually needs to do a lot more than this, but this is the portion relevant to PTO.
Fixes #3639